build: automatically generate list of library_files #3506

chrisdickinson · 2015-10-24T03:46:42Z

Automatically generate the list of library_files by using Python's os.walk so we don't have to remember to specify new files (or remember to remove old ones.)

Example output:

# Do not edit. Generated by the configure script.
{ 'target_defaults': { 'cflags': [],
                       'default_configuration': 'Release',
                       'defines': [],
                       'include_dirs': [],
                       'libraries': []},
  'variables': { 'asan': 0,
                 'host_arch': 'x64',
                 'icu_small': 'false',
                 'library_files': [ 'src/node.js',
                                    'lib/_debug_agent.js',
                                    'lib/_debugger.js',
                                    'lib/_http_agent.js',
                                    'lib/_http_client.js',
                                    'lib/_http_common.js',
                                    'lib/_http_incoming.js',
                                    'lib/_http_outgoing.js',
                                    'lib/_http_server.js',
                                    'lib/_linklist.js',
                                    'lib/_stream_duplex.js',
                                    'lib/_stream_passthrough.js',
                                    'lib/_stream_readable.js',
                                    'lib/_stream_transform.js',
                                    'lib/_stream_wrap.js',
                                    'lib/_stream_writable.js',
                                    'lib/_tls_common.js',
                                    'lib/_tls_legacy.js',
                                    'lib/_tls_wrap.js',
                                    'lib/assert.js',
                                    'lib/buffer.js',
                                    'lib/child_process.js',
                                    'lib/cluster.js',
                                    'lib/console.js',
                                    'lib/constants.js',
                                    'lib/crypto.js',
                                    'lib/dgram.js',
                                    'lib/dns.js',
                                    'lib/domain.js',
                                    'lib/events.js',
                                    'lib/freelist.js',
                                    'lib/fs.js',
                                    'lib/http.js',
                                    'lib/https.js',
                                    'lib/module.js',
                                    'lib/net.js',
                                    'lib/os.js',
                                    'lib/path.js',
                                    'lib/process.js',
                                    'lib/punycode.js',
                                    'lib/querystring.js',
                                    'lib/readline.js',
                                    'lib/repl.js',
                                    'lib/stream.js',
                                    'lib/string_decoder.js',
                                    'lib/sys.js',
                                    'lib/timers.js',
                                    'lib/tls.js',
                                    'lib/tty.js',
                                    'lib/url.js',
                                    'lib/util.js',
                                    'lib/v8.js',
                                    'lib/vm.js',
                                    'lib/zlib.js',
                                    'lib/internal/child_process.js',
                                    'lib/internal/freelist.js',
                                    'lib/internal/linkedlist.js',
                                    'lib/internal/module.js',
                                    'lib/internal/readme.md',
                                    'lib/internal/repl.js',
                                    'lib/internal/socket_list.js',
                                    'lib/internal/util.js',
                                    'lib/internal/streams/lazy_transform.js'],
                 'llvm_version': 0,
                 'node_byteorder': 'little',
                 'node_install_npm': 'true',
                 'node_prefix': '/usr/local',
                 'node_release_urlbase': '',
                 'node_shared_http_parser': 'false',
                 'node_shared_libuv': 'false',
                 'node_shared_openssl': 'false',
                 'node_shared_zlib': 'false',
                 'node_tag': '',
                 'node_use_dtrace': 'true',
                 'node_use_etw': 'false',
                 'node_use_lttng': 'false',
                 'node_use_openssl': 'true',
                 'node_use_perfctr': 'false',
                 'openssl_fips': '',
                 'openssl_no_asm': 0,
                 'python': '/usr/local/opt/python/bin/python2.7',
                 'target_arch': 'x64',
                 'uv_parent_path': '/deps/uv/',
                 'uv_use_dtrace': 'true',
                 'v8_enable_gdbjit': 0,
                 'v8_enable_i18n_support': 0,
                 'v8_no_strict_aliasing': 1,
                 'v8_optimized_debug': 0,
                 'v8_random_seed': 0,
                 'v8_use_snapshot': 1,
                 'want_separate_host_toolset': 0}}

thefourtheye · 2015-10-24T07:39:56Z

configure

+  o['variables']['library_files'] = modules
+  lib_dir = os.path.join(root_dir, 'lib')
+  for root, dirs, files in os.walk(lib_dir):
+    modules += map(lambda xs: os.path.join(root, xs)[2:], files)


Let us say that my editor creates a temporary file, then if I run ./configure, it will include that as well. Probably, we need to filter out the files which end with .js.

This should be addressed now. Good catch!

bnoordhuis · 2015-10-25T10:37:02Z

so we don't have to remember to specify new files (or remember to remove old ones.)

I don't really see the point. If you forget to add a new file, you'll find out soon enough because tests will start failing. Similarly, when you forget to remove a deleted file from the list, the build will fail at the j2sc step.

chrisdickinson · 2015-10-25T20:08:11Z

If you forget to add a new file, you'll find out soon enough because tests will start failing. Similarly, when you forget to remove a deleted file from the list, the build will fail at the j2sc step.

I'm mostly scratching an itch, here — the extra step of making sure the file was present in the gypfile is easy to forget (until you recompile and test), wasn't obvious when I was a newcomer, and presents a human-monitored style rule in PRs (I recall there was a comment about putting the files in the correct alphabetical order in the gypfile at some point?) It's definitely not a huge change, but I've run into it enough that I wanted to smooth this corner.

jbergstroem · 2015-10-25T22:34:09Z

I'm -0 to this PR but there's a pretty convincing argument regarding where to look/edit if you're actually adding a file to lib/. Can we improve feedback somehow instead?

jasnell · 2015-10-26T16:32:07Z

LGTM, but I'm +0.1 on whether it's needed ;-)

rvagg · 2015-10-28T04:31:49Z

heh, +0.1 for me too, that gets us to +0.2, lgtm

Fishrock123 · 2015-11-05T16:07:41Z

Uh, what's the reason to not do this programatically? SGTM? (I don't want to sign off because python tho)

Trott · 2016-02-28T07:06:05Z

@chrisdickinson Is this something you still want? Or would you be inclined to close this? (I have no opinion either way, just trying to tidy up inactive PRs a bit.)

jasnell · 2016-04-18T23:47:10Z

ping @chrisdickinson

jbergstroem · 2016-05-03T06:13:58Z

@Fishrock123 the counter argument would be that we're introducing an additional layer which down the road could be more error prone than just editing the file. Another (far fetched) argument would be that configure is not meant to be a preprocessor -- that's gyp's job.

Fishrock123 · 2016-05-03T19:04:13Z

Meh. The build will just fail if you don't edit it anyways. let's close unless someone wants to pick it up.

build: automatically generate list of library_files

d3c84db

chrisdickinson added the build Issues and PRs related to build files or the CI. label Oct 24, 2015

thefourtheye reviewed Oct 24, 2015
View reviewed changes

squash! make sure the extname is ".js"

7841bad

Trott force-pushed the master branch from 1e896a6 to 082cc8d Compare December 27, 2015 02:00

jasnell added the stalled Issues and PRs that are stalled. label Mar 22, 2016

Fishrock123 closed this May 3, 2016

snyk-bot mentioned this pull request Apr 10, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 jj-crypto/node#8

Open

terrorizer1980 mentioned this pull request Apr 21, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 terrorizer1980/node-1#9

Open

NOUIY mentioned this pull request Apr 21, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 NOUIY/node#91

Open

BryanLaura mentioned this pull request Jun 22, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 BryanLaura/node#25

Open

terrorizer1980 mentioned this pull request Jun 22, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 Exhorder6/node#11

Open

This was referenced Sep 16, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 trongham/node#31

Open

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 137717unity/node#67

Open

aliscco mentioned this pull request Sep 23, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 aliscco/alisco-node#65

Open

wedataintelligence mentioned this pull request Sep 26, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 wedataintelligence/node#42

Open

Verdinjoshua26 mentioned this pull request Sep 27, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 Verdinjoshua26/node#29

Open

yotamselementor mentioned this pull request Sep 27, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 yotamselementor/node#21

Open

snyk-bot mentioned this pull request Sep 27, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 Rednickle/node-1#57

Open

RoeeOren mentioned this pull request Oct 5, 2022

[Snyk] Fix for 2 vulnerabilities RoeeOren/node#33

Open

snyk-bot mentioned this pull request Oct 5, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 MrBrain295/node#9

Open

pratapadi mentioned this pull request Oct 5, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 pratapadi/node#29

Open

snyk-bot mentioned this pull request Oct 6, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 bsmoove247/node#38

Open

NOUIY mentioned this pull request Oct 9, 2022

[Snyk] Security upgrade d3 from 5.7.0 to 7.0.0 NOUIY/node#306

Open

snyk-bot mentioned this pull request Feb 16, 2023

[Snyk] Security upgrade d3 from 5.16.0 to 7.0.0 SanchitMahajan236/node#2

Merged

Renukaloch mentioned this pull request Aug 1, 2023

[Snyk] Security upgrade d3 from 5.16.0 to 7.0.0 Renukaloch/node#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: automatically generate list of library_files #3506

build: automatically generate list of library_files #3506

chrisdickinson commented Oct 24, 2015

thefourtheye Oct 24, 2015

chrisdickinson Oct 25, 2015

bnoordhuis commented Oct 25, 2015

chrisdickinson commented Oct 25, 2015

jbergstroem commented Oct 25, 2015

jasnell commented Oct 26, 2015

rvagg commented Oct 28, 2015

Fishrock123 commented Nov 5, 2015

Trott commented Feb 28, 2016

jasnell commented Apr 18, 2016

jbergstroem commented May 3, 2016

Fishrock123 commented May 3, 2016

build: automatically generate list of library_files #3506

build: automatically generate list of library_files #3506

Conversation

chrisdickinson commented Oct 24, 2015

thefourtheye Oct 24, 2015

Choose a reason for hiding this comment

chrisdickinson Oct 25, 2015

Choose a reason for hiding this comment

bnoordhuis commented Oct 25, 2015

chrisdickinson commented Oct 25, 2015

jbergstroem commented Oct 25, 2015

jasnell commented Oct 26, 2015

rvagg commented Oct 28, 2015

Fishrock123 commented Nov 5, 2015

Trott commented Feb 28, 2016

jasnell commented Apr 18, 2016

jbergstroem commented May 3, 2016

Fishrock123 commented May 3, 2016